Flink: Add decimal write/read roundtrip test for FlinkParquetReaders#16346
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Should we test this in the BaseFormatModelTests? |
…odelTests Per review feedback on apache#16346, instead of a Flink-only decimal Parquet test, add a StructWithDecimals generator to DataGenerators.ALL and a testDataWriterEngineWriteEngineRead case to BaseFormatModelTests. This exercises decimal(9,2)/(15,3)/(38,10) write-read roundtrips (Parquet INT32/INT64/FIXED_LEN_BYTE_ARRAY encodings) for Flink and Spark across Avro, Parquet and ORC, and closes the missing engine-write -> engine-read symmetry in the shared harness. The duplicated TestFlinkParquetReader decimal test is removed from Flink v1.20/v2.0/v2.1; the FlinkParquetReaders TODO stays resolved by the broader shared coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Good call, thanks @pvary — done in 6d2f777. Rather than a Flink-only test I added a One gap I hit while doing this: the generator-parameterized cases were only The duplicated |
| } | ||
| } | ||
|
|
||
| static class StructWithDecimals implements DataGenerator { |
There was a problem hiding this comment.
Good catch — there's no nested struct here, so StructWithDecimals was a misleading name. Renamed the class to Decimals (and updated the ALL array) in 5dc4b5d.
| static class StructWithDecimals implements DataGenerator { | ||
| private final Schema schema = | ||
| new Schema( | ||
| required(1, "row_id", Types.StringType.get()), |
There was a problem hiding this comment.
No — row_id isn't needed for the decimal write/read roundtrip and the generator has no identity semantics. Dropped the column and renumbered the decimal field IDs to 1/2/3 in 5dc4b5d.
…d column The generator has no nested struct, so StructWithDecimals was a misleading name; rename it to Decimals. The row_id String column is not needed for the decimal write/read roundtrip and carries no identity semantics, so drop it and renumber the decimal field IDs to 1/2/3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thank you for review! |
|
Merged to main. |
2 participants
Summary
FlinkParquetReaders.BinaryDecimalReaderby adding a Flink-writer → Flink-reader roundtrip test for decimals across precisions 9/2, 15/3 and 38/10 (covering INT32/INT64/FIXED_LEN_BYTE_ARRAY physical encodings).TestFlinkParquetReaderin v1.20, v2.0 and v2.1; the TODO comment is removed.